Add tvOS (Apple TV) support for NetBird VPN client #36
Conversation
This commit introduces full tvOS support for the NetBird iOS client,
enabling VPN connectivity on Apple TV devices.
- Added NetBird TV app target with tvOS 16.0+ deployment
- Created tvOS-specific UI using SwiftUI optimized for "10-foot experience"
- Tab-based navigation: Connection, Peers, Networks, Settings
- Large touch targets and text for Siri Remote navigation
- TVMainView: Main tab navigation and connection status
- TVConnectionView: Large connect/disconnect button with status display
- TVPeersView: List of connected peers with connection details
- TVNetworksView: Network routes selection and management
- TVSettingsView: App settings and logout functionality
- TVAuthView: QR code + device code authentication flow
- Implemented OAuth device authorization flow for tvOS
- Displays QR code that user scans with phone to authenticate
- Shows user code as fallback for manual entry
- Polls for authentication completion and auto-dismisses on success
tvOS has stricter sandbox restrictions than iOS:
1. **UserDefaults-based Config Storage**
- tvOS blocks file writes to App Group containers
- Config stored in shared UserDefaults instead of files
- Added Preferences methods: saveConfigToUserDefaults(),
loadConfigFromUserDefaults(), hasConfigInUserDefaults()
2. **Preloaded Config in Go SDK**
- SDK modified to accept config via setConfigFromJSON()
- Avoids file I/O that would fail in tvOS sandbox
- Config passed from UserDefaults to SDK at runtime
3. **Raw Syscall Tunnel FD Discovery**
- tvOS SDK doesn't expose ctl_info, sockaddr_ctl, CTLIOCGINFO
- Implemented findTunnelFileDescriptorTvOS() using raw memory ops
- Manually defines kernel structure layouts at byte level
- Uses getpeername() and ioctl() which ARE available on tvOS
- Added NetBirdTVNetworkExtension target
- Separate PacketTunnelProvider.swift with tvOS-specific handling
- Extensive logging for debugging via Console.app
- Handles "LoginTV" message for device auth flow
- Loads config from UserDefaults into SDK memory
- isLoginRequired() now verifies session with management server
- Previously only checked if config existed (caused post-restart failures)
- Shows QR code re-auth flow when OAuth session expires
- Added Platform.swift for iOS/tvOS conditional compilation
- Shared code uses #if os(tvOS) / #if os(iOS) where needed
- Common ViewModels work across both platforms
📝 WalkthroughWalkthroughAdds an Apple TV app and packet-tunnel network extension, cross-platform platform helpers, configuration/provider and IPC flows for tvOS (device-code SSO), extensive tvOS UI and theming, Xcode/CI/project wiring, entitlements, assets, and adapter/extension integration changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User as Apple TV User
participant TVApp as NetBird TV App
participant Pref as ConfigurationProvider / UserDefaults
participant Ext as PacketTunnel Extension
participant SDK as NetBird SDK
participant LoginSvc as Login Service
User->>TVApp: Start device-code login
TVApp->>Pref: loadConfigFromUserDefaults()
Pref-->>TVApp: configJSON
TVApp->>Ext: sendConfigToExtension(configJSON)
Ext->>SDK: initializeConfig / loginTV
SDK->>LoginSvc: Request device code
LoginSvc-->>SDK: DeviceCode + LoginURL
SDK-->>Ext: Return LoginURL + UserCode
Ext-->>TVApp: Config + userCode
TVApp->>User: Display QR and device code
loop Poll every 2s
TVApp->>Ext: checkLoginComplete()
Ext->>SDK: Check auth status
alt Auth complete
SDK-->>Ext: Success + config
Ext-->>TVApp: Login Complete
TVApp->>User: Close Auth UI
else Pending
Ext-->>TVApp: Not ready
end
end
sequenceDiagram
participant TVApp as NetBird TV App
participant Pref as ConfigurationProvider / UserDefaults
participant Ext as PacketTunnel Extension
participant SDK as NetBird SDK
participant VPN as NEPacketTunnelProvider
TVApp->>Pref: loadConfigFromUserDefaults()
Pref-->>TVApp: configJSON
TVApp->>Ext: sendConfigToExtension(configJSON)
Ext->>Ext: save config (UserDefaults / file fallback)
Ext->>SDK: NetBirdSDKNewClient(config)
SDK-->>Ext: client ready
Ext->>VPN: startTunnel()
VPN-->>Ext: Tunnel active
Ext-->>TVApp: Status update
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Update ServerViewModel to use new SDK callback-based API - Fix focus navigation on Networks and Settings tabs - Add white text on focus for better readability across all cards - Increase filter bar spacing to prevent highlight overlap - Add TVSettingsInfoRow for non-interactive display items
- Remove duplicate TVColors structs from 5 TV view files - Add ClearConfig IPC message to clear extension-local config on logout - Switch MainView from viewModel.isIpad to DeviceType.isPad - Remove unused isTV/isIpad properties from MainViewModel - Add TVColors.swift to Xcode project
- Remove dead shared UserDefaults fallback code in NetBirdAdapter - Document tvOS config storage architecture in Preferences.swift - Fix onChange deprecation warnings in TVSettingsView - Add "no peer auth method" detection in ServerViewModel
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (4)
NetBird/Source/App/NetBirdApp.swift (1)
45-53: Firebase configuration duplication noted.This tvOS initialization mirrors the iOS
AppDelegatesetup at lines 27-30. See the earlier comment for a suggested refactor to eliminate this duplication.NetBird.xcodeproj/xcshareddata/xcschemes/NetbirdNetworkExtension.xcscheme (1)
3-3: Consistent with main scheme update.The
LastUpgradeVersionbump to2610matches the update inNetBird.xcscheme. Same verification note applies regarding the version number.NetBird/Source/App/ViewModels/MainViewModel.swift (2)
264-290: This code appears to be dead/duplicated.Per past review comments, management server URL handling was migrated to
ServerViewModel. If this function is no longer used, it should be removed to avoid confusion.
307-324: This code appears to be dead/duplicated.Per past review comments, setup key handling was migrated to
ServerViewModel. If this function is no longer used, it should be removed.
🧹 Nitpick comments (23)
README.md (2)
101-101: Wrap the repository URL in markdown link syntax for consistency.The other repository entries in this section use markdown link syntax
[text](url), but this line uses a bare URL. Wrap the URL to maintain consistent formatting with the rest of the list.Apply this diff:
- iOS/tvOS Client: https://github.com/netbirdio/ios-client (this repository) + iOS/tvOS Client: [https://github.com/netbirdio/ios-client](https://github.com/netbirdio/ios-client) (this repository)
90-92: Minor: Consider rephrasing for variety.Line 88 uses "needs to be connected" and line 92 uses "needs to be paired"—both opening with "needs to be." Consider varying the phrasing in one of these sentences to improve readability.
NetBird/Source/App/NetBirdApp.swift (1)
19-34: Consider extracting Firebase configuration to reduce duplication.The Firebase setup logic is duplicated between the iOS
AppDelegate(lines 27-30) and the tvOSinit()method (lines 48-51). Consider extracting this into a shared helper method to improve maintainability. Additionally, silent failure when the plist is missing or invalid could complicate debugging—adding error logging would be helpful.Example refactor:
private func configureFirebase() { guard let path = Bundle.main.path(forResource: "GoogleService-Info", ofType: "plist"), let options = FirebaseOptions(contentsOfFile: path) else { print("Warning: GoogleService-Info.plist not found or invalid. Firebase not configured.") return } FirebaseApp.configure(options: options) }Then call
configureFirebase()from both the iOSAppDelegateand tvOSinit().NetbirdKit/Device.swift (1)
27-53: LGTM! Well-designed persistent device name generation.The implementation correctly generates a unique device name once and caches it for consistency across app launches. The force unwrap on
randomElement()is safe sincecharactersis a non-empty literal.Note:
defaults.synchronize()(line 48) is deprecated on modern iOS/tvOS as the system handles persistence automatically, but it's harmless here.NetbirdKit/Preferences.swift (1)
49-59: Redundant conditional—both branches are identical.The
#if os(tvOS)and#elsebranches execute the same code. Either remove the conditional or document why it's kept for future differentiation.static func newPreferences() -> NetBirdSDKPreferences? { - #if os(tvOS) - // On tvOS, creating SDK Preferences may fail if the app doesn't have write access - // to the App Group container. Try anyway - if it fails, settings will be managed - // via the extension instead. - // Note: The SDK now uses DirectWriteOutConfig which may work better on tvOS. - return NetBirdSDKNewPreferences(configFile(), stateFile()) - #else return NetBirdSDKNewPreferences(configFile(), stateFile()) - #endif }NetbirdNetworkExtension/NetBirdAdapter.swift (1)
461-484: Regex-based JSON manipulation is fragile.Using regex patterns like
"ManagementURL"\\s*:\\s*"([^"]*)"to parse/modify JSON can fail on escaped quotes, Unicode escapes, or whitespace variations. Consider usingJSONSerializationorCodable.+ /// Extract the management URL from a config JSON string using proper JSON parsing static func extractManagementURL(from configJSON: String) -> String? { - let pattern = "\"ManagementURL\"\\s*:\\s*\"([^\"]*)\"" - guard let regex = try? NSRegularExpression(pattern: pattern, options: []), - let match = regex.firstMatch(in: configJSON, options: [], range: NSRange(configJSON.startIndex..., in: configJSON)), - let urlRange = Range(match.range(at: 1), in: configJSON) else { + guard let data = configJSON.data(using: .utf8), + let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + let url = json["ManagementURL"] as? String, + !url.isEmpty else { return nil } - let url = String(configJSON[urlRange]) - return url.isEmpty ? nil : url + return url }NetbirdKit/NetworkExtensionAdapter.swift (4)
17-27: Duplicate listener class across codebase.
ConfigSSOListenerduplicates the same pattern asSSOCheckListenerinMainViewModel.swiftandSSOListenerImplinServerViewModel.swift. Consider extracting a single shared implementation inNetbirdKitto avoid maintaining multiple copies.
181-216:initializeConfigViaExtension()appears unused.This function is defined but never called. If it's intended for future use, consider marking it with a TODO or removing it to reduce dead code.
#!/bin/bash # Verify that initializeConfigViaExtension is not called anywhere rg -n "initializeConfigViaExtension" --type swift
247-269: SDK client created on everyisLoginRequired()call.On tvOS, a new
NetBirdSDKNewClientis instantiated each timeisLoginRequired()is called. This may be expensive if the SDK performs key generation or other heavy initialization. Consider caching the client or deferring this check.
333-335: Remove redundantVoidreturn type.Per SwiftLint, returning
Voidin a function declaration is redundant.- func stop() -> Void { + func stop() { self.vpnManager?.connection.stopVPNTunnel() }NetBird/Source/App/ViewModels/MainViewModel.swift (1)
18-28: Duplicate SSO listener pattern.
SSOCheckListenerduplicates the same pattern asConfigSSOListenerinNetworkExtensionAdapter.swiftandSSOListenerImplinServerViewModel.swift. Consider consolidating these into a single shared implementation.NetBird/Source/App/ViewModels/ServerViewModel.swift (1)
15-54: Well-designed listener implementations.
SSOListenerImplandErrListenerImpluse typed callbacks instead of optional closures, providing cleaner semantics. However, these duplicate similar listeners inMainViewModel.swiftandNetworkExtensionAdapter.swift.Consider consolidating all SDK listener implementations into a shared location in
NetbirdKitto reduce duplication across the codebase.NetBird/Source/App/Views/MainView.swift (1)
30-30: SwiftLint naming convention note.SwiftLint flags
iOSMainViewfor not starting with an uppercase character. WhileIOSMainViewwould satisfy the linting rule,iOSMainViewfollows Apple's branding convention (iOS, iPhone, iPad) and is more readable. Consider either:
- Keeping
iOSMainViewand suppressing the lint warning with// swiftlint:disable:next type_name- Renaming to
IOSMainViewfor strict conformanceIf you choose to keep the current naming (recommended for readability), add a suppression comment:
+// swiftlint:disable:next type_name struct iOSMainView: View {NetBird/Source/App/Views/PeerTabView.swift (1)
196-225: Platform-specific context menu is well-implemented.The iOS clipboard actions and tvOS static text fallback are correctly guarded. Consider wrapping the debug
#if DEBUGfor consistency with the logging approach used elsewhere in this PR.Button("Copy FQDN") { UIPasteboard.general.string = peer.fqdn + #if DEBUG print("Copied FQDN to clipboard") + #endif viewModel.showFqdnCopiedAlert = true ... } Button("Copy IP") { UIPasteboard.general.string = peer.ip + #if DEBUG print("Copied IP to clipboard") + #endif viewModel.showIpCopiedAlert = true ... }NetBird/Source/App/Views/TV/TVNetworksView.swift (3)
11-12: Remove unusedUIKitimport.The
UIKitimport doesn't appear to be used in this file. All types used are from SwiftUI.import SwiftUI -import UIKit
121-128: Refresh animation duration is not tied to actual data fetch completion.The 2-second delay is arbitrary and doesn't reflect when
getRoutes()actually completes. Consider using a completion handler fromgetRoutes()to stop the animation when data is ready.private func refresh() { isRefreshing = true - viewModel.routeViewModel.getRoutes() - - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { + viewModel.routeViewModel.getRoutes { isRefreshing = false } }This would require adding a completion parameter to
getRoutes()inRoutesViewModel.
190-198: Magic string"invalid Prefix"is fragile.The comparison against
"invalid Prefix"relies on an exact string match from another layer. Consider defining this as a constant or using a more robust check (e.g., an enum or dedicated property on the route model).NetBird/Source/App/Views/TV/TVSettingsView.swift (1)
141-185: Consider makingactionnon-optional or using a different pattern.The
TVSettingsRowaccepts an optionalactionand disables the button whenaction == nil(line 183). However, using a non-focusable info row (TVSettingsInfoRow) for display-only items would be more semantically correct. The current pattern could confuse users who focus on a row that does nothing.If you want to keep this pattern, consider providing clearer visual feedback that the row is disabled:
.disabled(action == nil) +.opacity(action == nil ? 0.5 : 1.0)NetBird/Source/App/Platform/Platform.swift (1)
182-189:tvFocusable()modifier applies.focusable()unconditionally on tvOS.On tvOS,
.focusable()makes a view focusable for the Siri Remote. This is correct, but note that applying this to views that are already naturally focusable (likeButton) is redundant. Consider documenting the intended use case (e.g., for non-interactive views that need focus).NetBird/Source/App/Views/TV/TVPeersView.swift (1)
27-28: String comparison for connection state is fragile.Comparing
viewModel.extensionStateText == "Connected"relies on a magic string. If the state text changes (e.g., localization), this check will break silently. Consider using an enum or constant for state comparison.- if viewModel.extensionStateText == "Connected" && + if viewModel.isConnected && viewModel.peerViewModel.peerInfo.count > 0 {If
isConnecteddoesn't exist on the ViewModel, consider adding it as a computed property that encapsulates this check.NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
219-238:UserDefaults.synchronize()is deprecated.
synchronize()is deprecated and generally unnecessary as the system handles persistence automatically. While it might seem safer for IPC scenarios, it can actually cause performance issues.// Save to extension-local UserDefaults (not shared App Group) UserDefaults.standard.set(configJSON, forKey: "netbird_config_json_local") - UserDefaults.standard.synchronize()Also applies to line 252.
613-657: Redundant file clearing before writing ininitializeLogging.Lines 630-639 open the file for writing, clear it with an empty string write, then write the log message. This is overly complex. Simply truncating and writing in one step would be cleaner.
if fileManager.fileExists(atPath: logURLValid.path) { - if let fileHandle = try? FileHandle(forWritingTo: logURLValid) { - do { - try "".write(to: logURLValid, atomically: true, encoding: .utf8) - } catch { - print("Error handling the log file: \(error)") - } - if let data = logMessage.data(using: .utf8) { - fileHandle.write(data) - } - fileHandle.closeFile() - } else { - print("Failed to open the log file for writing.") - } + do { + try logMessage.write(to: logURLValid, atomically: true, encoding: .utf8) + } catch { + print("Failed to overwrite the log file: \(error.localizedDescription)") + } } else {This simplifies the existing-file case to match the new-file case.
619-620: Unused variableserrorandsuccess.The variables
error(line 619) andsuccess(line 620) are declared early but only used much later (lines 652-656). Consider moving their declarations closer to usage for clarity, or removing them if the success/error logging isn't critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/netbird-tvos-icon.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/netbird-tvos-icon.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Content.imageset/netbird-tvos-icon.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/icon-netbird-button.imageset/icon-netbird-button.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/icon-netbird-button.imageset/[email protected]is excluded by!**/*.pngNetBird TV/Assets.xcassets/icon-netbird-button.imageset/[email protected]is excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu 1.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu@2x 1.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/[email protected]is excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu@3x 1.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/[email protected]is excluded by!**/*.png
📒 Files selected for processing (60)
NetBird TV/Assets.xcassets/AccentColor.colorset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/Contents.json(1 hunks)NetBird TV/Assets.xcassets/icon-netbird-button.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/netbird-logo-menu.imageset/Contents.json(1 hunks)NetBird TV/ContentView.swift(1 hunks)NetBird TV/NetBird TVDebug.entitlements(1 hunks)NetBird.xcodeproj/project.pbxproj(25 hunks)NetBird.xcodeproj/xcshareddata/xcschemes/NetBird.xcscheme(1 hunks)NetBird.xcodeproj/xcshareddata/xcschemes/NetbirdNetworkExtension.xcscheme(1 hunks)NetBird/Source/App/NetBirdApp.swift(1 hunks)NetBird/Source/App/Platform/Platform.swift(1 hunks)NetBird/Source/App/ViewModels/MainViewModel.swift(9 hunks)NetBird/Source/App/ViewModels/PeerViewModel.swift(1 hunks)NetBird/Source/App/ViewModels/RoutesViewModel.swift(1 hunks)NetBird/Source/App/ViewModels/ServerViewModel.swift(3 hunks)NetBird/Source/App/Views/Components/SafariView.swift(2 hunks)NetBird/Source/App/Views/Components/SideDrawer.swift(2 hunks)NetBird/Source/App/Views/MainView.swift(4 hunks)NetBird/Source/App/Views/PeerTabView.swift(5 hunks)NetBird/Source/App/Views/RouteTabView.swift(3 hunks)NetBird/Source/App/Views/ServerView.swift(1 hunks)NetBird/Source/App/Views/TV/TVAuthView.swift(1 hunks)NetBird/Source/App/Views/TV/TVColors.swift(1 hunks)NetBird/Source/App/Views/TV/TVMainView.swift(1 hunks)NetBird/Source/App/Views/TV/TVNetworksView.swift(1 hunks)NetBird/Source/App/Views/TV/TVPeersView.swift(1 hunks)NetBird/Source/App/Views/TV/TVServerView.swift(1 hunks)NetBird/Source/App/Views/TV/TVSettingsView.swift(1 hunks)NetBirdTV/Info.plist(1 hunks)NetBirdTV/NetBirdTV.entitlements(1 hunks)NetBirdTV/NetBirdTVNetworkExtension.entitlements(1 hunks)NetBirdTVNetworkExtension/Info.plist(1 hunks)NetBirdTVNetworkExtension/NetBirdTVNetworkExtension.entitlements(1 hunks)NetBirdTVNetworkExtension/NetBirdTVNetworkExtensionDebug.entitlements(1 hunks)NetBirdTVNetworkExtension/PacketTunnelProvider.swift(1 hunks)NetbirdKit/ConnectionListener.swift(1 hunks)NetbirdKit/DNSManager.swift(1 hunks)NetbirdKit/Device.swift(1 hunks)NetbirdKit/EnvVarPackager.swift(1 hunks)NetbirdKit/NetworkExtensionAdapter.swift(6 hunks)NetbirdKit/Preferences.swift(1 hunks)NetbirdKit/RoutesSelectionDetails.swift(1 hunks)NetbirdKit/StatusDetails.swift(1 hunks)NetbirdNetworkExtension/NetBirdAdapter.swift(5 hunks)README.md(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
NetBird/Source/App/Views/RouteTabView.swift (1)
NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)
NetBird TV/ContentView.swift (1)
NetBird/Source/App/Views/Components/TransparentGradientButton.swift (1)
body(22-53)
NetBird/Source/App/Views/TV/TVServerView.swift (4)
NetbirdKit/Preferences.swift (1)
configFile(61-66)NetbirdKit/Device.swift (1)
getName(11-17)NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
clearErrorsFor(298-310)loginWithSetupKey(229-296)changeManagementServerAddress(144-206)
NetBird/Source/App/Views/TV/TVNetworksView.swift (4)
NetbirdKit/NetworkExtensionAdapter.swift (1)
getRoutes(478-509)NetBird/Source/App/ViewModels/RoutesViewModel.swift (3)
getRoutes(55-60)deselectRoute(76-82)selectRoute(62-68)NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
deselectRoute(586-593)selectRoute(577-584)
NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
onSuccess(20-22)onError(24-26)login(209-214)NetbirdKit/NetworkExtensionAdapter.swift (3)
onSuccess(20-22)onError(24-26)login(337-380)NetbirdNetworkExtension/NetBirdAdapter.swift (5)
onSuccess(40-42)onSuccess(49-51)onError(36-38)onError(53-55)login(310-313)
NetbirdNetworkExtension/NetBirdAdapter.swift (3)
NetbirdKit/Device.swift (3)
getName(11-17)getOsVersion(19-21)getOsName(23-25)NetbirdKit/Preferences.swift (3)
configFile(61-66)stateFile(68-73)saveConfigToUserDefaults(86-93)NetbirdKit/EnvVarPackager.swift (1)
getEnvironmentVariables(12-29)
NetBird/Source/App/Views/PeerTabView.swift (1)
NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)
NetbirdKit/NetworkExtensionAdapter.swift (2)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (4)
onSuccess(20-22)onError(24-26)login(209-214)checkLoginComplete(356-390)NetbirdKit/Preferences.swift (4)
loadConfigFromUserDefaults(96-101)configFile(61-66)stateFile(68-73)hasConfigInUserDefaults(104-109)
NetBird/Source/App/Views/TV/TVSettingsView.swift (1)
NetBird/Source/App/ViewModels/MainViewModel.swift (3)
setRosenpassEnabled(368-379)close(168-178)clearDetails(292-305)
🪛 LanguageTool
README.md
[style] ~92-~92: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...sical device running tvOS 17.0 or later needs to be [paired with Xcode](https://support....
(REP_NEED_TO_VB)
🪛 markdownlint-cli2 (0.18.1)
README.md
101-101: Bare URL used
(MD034, no-bare-urls)
🪛 SwiftLint (0.57.0)
NetBird/Source/App/Views/MainView.swift
[Error] 30-30: Type name 'iOSMainView' should start with an uppercase character
(type_name)
NetbirdKit/NetworkExtensionAdapter.swift
[Warning] 333-333: Returning Void in a function declaration is redundant
(redundant_void_return)
🔇 Additional comments (82)
README.md (1)
31-33: Documentation update comprehensively covers tvOS support.The README expansions effectively communicate the new tvOS capability with clear build instructions, platform requirements, and device setup guidance. The separation of build commands for iOS-only vs. iOS+tvOS scenarios is particularly helpful for developers.
Also applies to: 57-59, 70-82, 86-92
NetBird/Source/App/ViewModels/RoutesViewModel.swift (1)
8-8: Explicit Foundation import is appropriate.Adding the explicit
import Foundationstatement aligns with best practices for cross-platform integration and ensures clarity of dependencies. This is a suitable change for the tvOS support refactoring.NetBird/Source/App/Views/Components/SideDrawer.swift (2)
7-13: LGTM! Clear platform separation.The comments effectively document the iOS-only nature of the side drawer pattern and reference the tvOS alternative (TVSettingsView). The
#if os(iOS)guard properly isolates this iOS-specific navigation pattern.
172-172: LGTM! Properly closes the platform guard.The
#endifcorrectly closes the iOS compilation block, and the inline comment helps readability. The entire SideDrawer and SideMenu implementation (including UIKit-dependent code likeUIDevice.current.userInterfaceIdiomandUIScreen.main.bounds) is appropriately isolated to iOS.NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Content.imageset/Contents.json (1)
1-11: Clarify intent for the incomplete tvOS imagestack structure.The imagestack (Back, Middle layers) contains valid but minimal Contents.json files with no image filenames and no actual image asset files. Confirm whether this is:
- An intentional placeholder for future image additions
- An incomplete structure that should include image files now
(Note: Both Back and Middle layers have identical JSON structure with no filename references, and no image files exist anywhere in this imagestack.)
NetBird/Source/App/Views/Components/SafariView.swift (1)
1-11: LGTM! Proper platform gating for iOS-only SafariServices.The conditional compilation block correctly restricts SafariServices usage to iOS builds, which is necessary since this framework is unavailable on tvOS. The header comment clearly documents the iOS-only nature of this component.
NetBird/Source/App/NetBirdApp.swift (2)
14-17: LGTM: Clean conditional compilation for iOS-only Firebase Performance.The conditional import is appropriate since Firebase Performance is not available on tvOS.
59-85: Reconsider unifying lifecycle approaches—behavioral equivalence is not confirmed.iOS uses
NotificationCenterwithUIApplicationlifecycle notifications (lines 60, 65), while tvOS usesscenePhaseobservation (lines 72-84). The main difference is that onReceive hooks into UIKit's notification system, while scenePhase is SwiftUI's built-in, declarative alternative. However, these approaches may not be behaviorally equivalent. ScenePhase has a number of bugs (or at least, unexpected behavior) and is limited to only 3 states: active, inactive, and background. Verify timing alignment and edge cases between the two approaches before unifying them, as the UIApplication notifications and scenePhase changes may not fire at identical moments.NetBird TV/Assets.xcassets/Contents.json (1)
1-6: LGTM! Standard asset catalog metadata.This is a standard Xcode-generated asset catalog root configuration file with correct format and metadata.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Contents.json (1)
1-14: LGTM! Standard App Store app icon imagestack configuration.The layer structure and metadata follow Apple's tvOS asset catalog conventions correctly.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/Contents.json (1)
1-16: LGTM! Correct tvOS Top Shelf Wide image configuration.The asset correctly uses the "tv" idiom and provides appropriate scale variants (1x, 2x) for tvOS Top Shelf imagery.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Contents.json (1)
1-6: LGTM! Standard imagestack layer metadata.This is a correctly formatted layer container metadata file following Xcode asset catalog conventions.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/Contents.json (1)
1-16: LGTM! Correct tvOS Top Shelf image configuration.The asset correctly uses the "tv" idiom and provides appropriate scale variants for tvOS Top Shelf imagery.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json (1)
1-11: LGTM! Standard app icon layer image content.The configuration correctly uses the "tv" idiom for the App Store app icon middle layer content.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Contents.json (1)
1-6: LGTM! Standard imagestack layer metadata.This is a correctly formatted layer container metadata file following Xcode asset catalog conventions.
NetBird TV/Assets.xcassets/icon-netbird-button.imageset/Contents.json (1)
1-23: LGTM! Standard cross-platform button icon configuration.The asset correctly uses the "universal" idiom and provides all standard scale variants (1x, 2x, 3x) with proper naming conventions. This allows the icon to be shared across iOS and tvOS targets.
NetBird TV/Assets.xcassets/AccentColor.colorset/Contents.json (1)
1-11: Standard asset catalog structure.This is a correctly formatted Xcode asset catalog definition for AccentColor. Since no specific color values are defined, the app will use the system default accent color, which is a valid design choice.
NetBird TV/Assets.xcassets/netbird-logo-menu.imageset/Contents.json (1)
1-56: Well-structured asset catalog with complete variants.This asset definition properly includes all standard scales (1x, 2x, 3x) with both light and dark appearance variants, ensuring the logo displays correctly across all device resolutions and appearance modes.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Contents.json (1)
1-6: Standard metadata structure for empty layer.This minimal structure is appropriate for the back layer of the App Store icon imagestack, which typically doesn't require visible content.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Contents.json (1)
1-17: Correctly structured tvOS image stack.The three-layer structure (Front, Middle, Back) follows Apple's tvOS parallax icon conventions and properly coordinates the individual layer assets.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Contents.json (1)
1-6: Standard layer metadata structure.This metadata file correctly provides Xcode authoring information for the Middle image stack layer, coordinating with the nested Content.imageset.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/Contents.json (1)
1-17: Asset catalog structure looks correct.The JSON structure is valid for a tvOS Content.imageset. The 1x scale references
netbird-tvos-icon.png, and the 2x scale entry (without a filename) is acceptable when a @2x asset is not provided.Note: The AI summary incorrectly describes this file as being for the "Front.imagestacklayer", but the actual file path indicates it's for the "Back.imagestacklayer".
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Contents.json (1)
1-32: LGTM! tvOS asset catalog structure is correct.The brandassets Contents.json properly defines all required tvOS assets with correct idioms, roles, and dimensions:
- Primary app icons at both App Store (1280x768) and standard (400x240) sizes
- Top Shelf images in both wide (2320x720) and standard (1920x720) formats
This follows Apple's tvOS asset catalog requirements.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Contents.json (1)
1-6: File structure is correct—imagestacklayer Contents.json is metadata-only.The layer-level Contents.json file contains only metadata, which is the correct structure. Image definitions belong in the nested
Content.imageset/Contents.jsonfile (which already has theimagesarray with appropriate entries). All comparable imagestacklayer Contents.json files in this project follow the same metadata-only pattern.Likely an incorrect or invalid review comment.
NetbirdKit/ConnectionListener.swift (1)
9-9: LGTM!The
NetBirdSDKimport is required sinceConnectionListenerconforms toNetBirdSDKConnectionListenerProtocol.NetBird.xcodeproj/xcshareddata/xcschemes/NetBird.xcscheme (1)
3-3: Verify the Xcode version in scheme metadata.The
LastUpgradeVersionof2610appears unusually high. Xcode version numbers typically follow the format where 1430 = Xcode 14.3 and 1610 = Xcode 16.1. Version2610would imply Xcode 26.1, which doesn't exist.If this was generated by Xcode automatically, it's harmless. Otherwise, confirm this wasn't a typo (perhaps intended
1610for Xcode 16.1?).NetbirdKit/DNSManager.swift (1)
9-9: LGTM!The
NetBirdSDKimport is required sinceDNSManagerconforms toNetBirdSDKDnsManagerProtocol.NetBird/Source/App/ViewModels/PeerViewModel.swift (1)
8-9: LGTM!The explicit
Foundationimport improves clarity, even thoughCombinere-exports it. The view model logic is clean and well-structured.NetbirdKit/EnvVarPackager.swift (1)
17-25: LGTM! Platform-specific relay defaults are sensible.Defaulting to forced relay on iOS while allowing direct connections on tvOS aligns with the different network environments of these platforms.
NetBirdTV/NetBirdTVNetworkExtension.entitlements (1)
1-19: LGTM! Entitlements are correctly configured.The
packet-tunnel-providercapability andgroup.io.netbird.app.tvapp group are appropriate for the tvOS network extension.NetbirdKit/RoutesSelectionDetails.swift (1)
1-8: LGTM!Standard file header and imports for Combine-backed observable models.
NetbirdKit/StatusDetails.swift (1)
9-9: LGTM!The
Combineimport is required sincePeerInfoconforms toObservableObject.NetbirdNetworkExtension/NetBirdAdapter.swift (2)
133-214: Thorough low-level implementation for tvOS tunnel discovery.The raw memory operations correctly replicate the kernel structure layout for
ctl_infoandsockaddr_ctl. The hardcoded sizes (100 bytes for ctl_info, 32 bytes for sockaddr_ctl) and offsets match the Darwin kernel ABI which is stable.Good use of logging via
adapterLoggerfor debugging tunnel discovery issues on tvOS.
321-451: Well-structured async login flow with proper completion tracking.The
NSLock-based completion guard correctly prevents duplicate callbacks, and the capturedauthRefensures config can be saved after successful authentication. The separation ofhandleSuccess/handleErrorclosures keeps the logic clean.NetbirdKit/NetworkExtensionAdapter.swift (3)
356-369: tvOS response parsing handles missing userCode gracefully.The code correctly falls back to using the whole string if the expected
|separator is not found. Good defensive handling.
385-428: Well-structured diagnostic response parsing.The
checkLoginCompletemethod handles multiple response formats gracefully with progressive fallbacks, which is good for backward compatibility.
596-665: tvOS config IPC methods are well-implemented.
sendConfigToExtensionandclearExtensionConfigproperly handle session availability, encoding errors, and provide completion callbacks. Good error logging throughout.NetBird.xcodeproj/project.pbxproj (1)
1341-1341: Different development team for Release configuration.The iOS Release configuration uses
DEVELOPMENT_TEAM = 94333M4JTAwhile Debug usesTA739QLA7A. Ensure this is intentional (e.g., different teams for development vs distribution).NetBird/Source/App/ViewModels/MainViewModel.swift (4)
104-115: Good optimization with lazy preferences loading.Deferring SDK initialization until first access avoids blocking app startup, especially important on tvOS where key generation is expensive. Well-documented with comments.
149-166: Connect flow is well-structured.The button lock with 3-second timeout, logging, and async Task handling is appropriate for VPN connection management.
240-258: tvOS-specific state text updates are well-implemented.The switch statement correctly maps VPN states to user-friendly text and triggers route fetching when connected. Good use of platform-specific compilation.
297-305: Good cleanup for tvOS config on logout.Clearing both UserDefaults config and extension-local config ensures no stale credentials remain after logout.
NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
164-205: Good use of checked continuation for async bridging.The
withCheckedContinuationpattern correctly bridges the callback-based SDK API to async/await. The tvOS-specific handling of file permission errors by falling back to UserDefaults is a pragmatic workaround.
208-227: tvOS config persistence fallback is well-implemented.The
saveConfigToUserDefaultshelper correctly extracts config JSON from the authenticator and persists it when file writes are blocked. Error logging is appropriate.
259-295: Login with setup key handles tvOS constraints appropriately.Same pattern as
changeManagementServerAddresswith proper continuation bridging and tvOS file permission error handling.NetBird/Source/App/Views/ServerView.swift (1)
272-275: No functional changes.Only trailing whitespace adjustment. LGTM.
NetBird TV/NetBird TVDebug.entitlements (1)
1-14: LGTM! Standard tvOS network extension entitlements.The entitlements correctly declare the packet-tunnel-provider capability and app group for IPC between the tvOS app and its network extension. The configuration follows Apple's standard pattern for VPN-enabled apps.
NetBirdTV/NetBirdTV.entitlements (2)
1-18: LGTM! Entitlements properly configured for tvOS VPN app.The entitlements correctly declare packet-tunnel-provider and app group capabilities, matching the debug entitlements and following Apple's standard pattern.
5-5: Confirm tvOS 17.0 is the correct minimum version requirement.The code consistently specifies tvOS 17.0 as the minimum: NetBirdTV/Info.plist sets MinimumOSVersion to "17.0" with the comment "17.0 required for VPN support" (line 13), and NetBirdTV/NetBirdTV.entitlements confirms this with "requires tvOS 17+" (line 5). If the PR description claims "tvOS 16.0+ deployment", it should be corrected to reflect the actual tvOS 17.0+ requirement.
NetBirdTVNetworkExtension/NetBirdTVNetworkExtensionDebug.entitlements (1)
1-14: LGTM! Network extension debug entitlements properly configured.The entitlements correctly match the main app's app group identifier (group.io.netbird.app.tv), enabling proper IPC between the tvOS app and its network extension.
NetBirdTVNetworkExtension/Info.plist (1)
1-13: LGTM! Standard packet tunnel extension configuration.The Info.plist correctly defines the extension point identifier and principal class using the standard pattern. The $(PRODUCT_MODULE_NAME) variable will be resolved at build time to reference the PacketTunnelProvider implementation.
NetBirdTV/Info.plist (2)
13-15: MinimumOSVersion set to 17.0.This aligns with the VPN/Network Extension requirement noted in the entitlements, but conflicts with the PR summary stating "tvOS 16.0+ deployment". This version requirement was already flagged in the entitlements review.
1-27: LGTM! tvOS app Info.plist properly configured.The configuration is appropriate for a tvOS utilities app with automatic UI style support. The bundle display name, category, and launch screen setup follow Apple's standard patterns.
NetBirdTVNetworkExtension/NetBirdTVNetworkExtension.entitlements (1)
1-14: LGTM! Network extension entitlements consistent across targets.The app group identifier (group.io.netbird.app.tv) is consistent across all tvOS targets (app and extension, debug and release), ensuring proper IPC functionality.
NetBird/Source/App/Views/RouteTabView.swift (2)
86-92: LGTM! Platform-agnostic layout implementation.The refactoring from UIScreen.main.bounds to Screen helper and Layout.fontScale properly abstracts platform-specific sizing, enabling code sharing between iOS and tvOS while maintaining appropriate layout for each platform.
107-110: LGTM! Consistent use of platform abstraction.The padding calculations consistently use Screen helper for cross-platform compatibility.
NetBird/Source/App/Views/MainView.swift (3)
13-27: LGTM! Clean platform-specific routing implementation.The MainView correctly acts as a platform router, delegating to TVMainView for tvOS and iOSMainView for iOS. The conditional compilation ensures each platform only compiles its relevant code, maintaining clean separation.
75-75: LGTM! Platform-agnostic sizing with device type detection.The refactoring correctly uses Screen.height and DeviceType.isPad for adaptive layout that works across iOS device types (iPhone/iPad) while being compatible with the broader platform abstraction supporting tvOS.
Also applies to: 85-85
29-566: LGTM! iOS-specific code properly isolated.The #if os(iOS) conditional compilation correctly isolates iOS-specific UI code, ensuring it doesn't interfere with tvOS builds while maintaining the full iOS implementation.
NetBird/Source/App/Views/PeerTabView.swift (2)
7-15: LGTM!The conditional UIKit import and documentation comments are well-structured. This correctly limits UIKit usage to iOS builds where
UIPasteboardis actually used.
81-89: Good adoption of cross-platform sizing abstraction.The migration from
UIScreen-based sizing toScreen.*metrics aligns well with thePlatform.swiftabstraction for consistent behavior across iOS and tvOS.NetBird/Source/App/Views/TV/TVAuthView.swift (3)
1-19: Well-documented device code flow implementation.Good documentation explaining the authentication pattern. The QR code approach is appropriate for tvOS where Safari isn't available.
214-231: QR code generation looks correct.The use of
CIFilter.qrCodeGenerator()with appropriate scaling for tvOS visibility is well-implemented.
192-198: Good timer lifecycle management.Timer is correctly started on
onAppearand invalidated ononDisappear, preventing memory leaks.NetBird/Source/App/Views/TV/TVMainView.swift (4)
24-55: Clean TabView navigation structure.The tab-based navigation with proper tags and environment object passing is well-implemented for the tvOS 10-foot experience.
62-99: Auth flow integration with TVAuthView is complete.All necessary callbacks (
onCancel,onComplete,onError,checkLoginComplete,checkLoginError) are properly wired to the network extension adapter.
267-282: Connection button logic handles transitional states.The
handleTapfunction correctly handles both "Connected" and "Connecting" states for disconnection, allowing users to cancel an in-progress connection attempt. The logger usage provides good observability.
212-240: Good tvOS focus state handling.The
@FocusStateusage with scale animation provides appropriate visual feedback for Siri Remote navigation. The button is correctly disabled whenbuttonLockis true.NetBird/Source/App/Views/TV/TVServerView.swift (2)
25-28: ServerViewModel initialization looks correct.The view model is properly initialized with the config file path and device name, matching the existing iOS implementation patterns.
305-329: Server change logic is well-structured.The function correctly handles the three cases: empty inputs, URL-only, and URL with setup key. The
Task.yield()allows UI updates before async operations.NetBird/Source/App/Views/TV/TVNetworksView.swift (2)
200-206: Toggle route implementation is clean.The delegation to
routeViewModel.selectRoute/deselectRouteis straightforward and matches the existing RoutesViewModel API.
17-36: TVNetworksView structure is well-organized.The conditional rendering based on connection state and route availability follows the established pattern from other TV views.
NetBird/Source/App/Views/TV/TVColors.swift (2)
18-57: Well-structured color abstraction with safe fallbacks.The
colorOrFallbackhelper pattern ensures graceful degradation when asset catalog colors are missing. This is a clean approach for tvOS theming.
63-165: Layout constants are comprehensive and well-documented.The constants cover all major UI aspects (padding, spacing, typography, focus effects) with appropriate values for TV viewing distances. Consider extracting these to a protocol or making them configurable if you anticipate supporting multiple TV form factors in the future.
NetBird/Source/App/Views/TV/TVSettingsView.swift (2)
17-116: Clean two-pane layout with proper EnvironmentObject usage.The settings view structure is well-organized with sections for Connection, Advanced, and Info. The binding for Rosenpass using a custom getter/setter (lines 57-60) correctly delegates to the ViewModel's
setRosenpassEnabledmethod.
268-359: Focus management in the alert is well-implemented.The
onChangehandler (lines 349-357) that pulls focus back when it escapes is a good pattern for tvOS modal dialogs. The_ = oldValueon line 350 to suppress the unused warning is acceptable, though you could also useonChange(of: focusedButton) { newValue in ... }if targeting older tvOS versions isn't a concern.NetBird/Source/App/Platform/Platform.swift (2)
15-48: tvOS screen dimensions are hardcoded to 1920x1080 points.The comment mentions "3840x2160 for 4K, but points are same" - this is correct for tvOS where the point-based coordinate system uses 1920x1080 regardless of actual resolution. However, note that these are static properties computed at call time, which is fine for tvOS but on iOS, screen dimensions could potentially change (e.g., during Split View on iPad).
91-135: VPN support via Network Extension was added in tvOS 17.0.tvOS 17 added support for third-party VPN apps through Network Extensions. The code correctly checks for tvOS 17.0+ before enabling VPN support. The Network Extension framework allows developers to create custom VPN solutions for tvOS. The capability flags are well-structured and the comments explaining tvOS limitations are helpful.
NetBird/Source/App/Views/TV/TVPeersView.swift (2)
114-184: TVPeerCard is well-implemented with proper focus handling.The focus-driven styling, scale animation, and selection state management are appropriate for tvOS. The status color mapping (lines 177-183) covers the expected states.
244-286: Filter bar with focus-aware styling looks good.The
TVFilterBarandTVFilterButtoncomponents handle focus states well with appropriate visual feedback (scale, color changes).NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
17-27: ConfigInitSSOListener handles both success and error paths correctly.The listener properly propagates results via the closure callback. The optional typing for
onResultis appropriate.
395-470: Completion handler may be called multiple times inloginTV.The
completionHandleris called inonURLcallback (line 429), and could potentially be called again inonErrorifurlSentToAppcheck fails due to a race condition (though the lock helps). More critically, the NEPacketTunnelProvider'shandleAppMessagecompletion handler should only be called once. The current logic is safe due to the lock, but consider documenting this invariant clearly.Verify that the
onURLcallback is guaranteed to be called beforeonSuccess/onError, and that only one of success/error is ever called. If bothonURLandonSuccesscan trigger in sequence, the current design is correct. However, if there's any scenario whereonErrorcould be called beforeonURLand thenonURLis called afterward, you'd have a problem.
...f Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/Contents.json
Show resolved
Hide resolved
... Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json
Show resolved
Hide resolved
…ndling - Fix mismatched bundle IDs between Debug/Release configs for tvOS targets - Make Preferences.configFile() and stateFile() return optionals instead of force-unwrapping, with proper error logging when app group unavailable - Make NetBirdAdapter.init failable to handle SDK client creation failures - Fail fast in NetBirdAdapter.start() when tunnel file descriptor is invalid instead of silently passing fd=0 (stdin) to the SDK - Ensure all handleAppMessage switch cases call completionHandler to prevent IPC callers from hanging indefinitely
(functionality migrated to ServerViewModel) - Remove orphaned managementURL property - Simplify supportsKeyboard to just return true (both branches were identical) - Updated README instructions on buiilding tvOS SDK (needs NetBird gomobile fork)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
NetbirdNetworkExtension/PacketTunnelProvider.swift (1)
106-117: Completion handlers now called consistently.The Select/Deselect and default cases now invoke the completion handler, which resolves the IPC contract issue flagged in previous reviews.
Optional: Consider non-optional Data initializer.
SwiftLint suggests using
Data("true".utf8)instead of"true".data(using: .utf8)for lines 109 and 113. The non-optional initializer avoids unnecessary optionals when the encoding is guaranteed to succeed.NetBirdTVNetworkExtension/PacketTunnelProvider.swift (1)
118-166: LGTM: Comprehensive IPC message handling for tvOS.The handleAppMessage implementation covers all the necessary IPC commands for tvOS, including config initialization, device auth login, and config transfer. The completion handlers are now consistently called for all message types, which addresses the previous review concern.
Optional: Consider non-optional Data initializer for consistency.
Lines 149, 153, 257, 276, 285, 294, 314, 324, 331, 335, 388, and 424 all use
"string".data(using: .utf8)which returns an optional. SwiftLint recommendsData("string".utf8)which is non-optional and more concise. This is a minor style improvement.
🧹 Nitpick comments (4)
README.md (1)
96-96: Reduce repetitive phrasing for improved readability.The phrase "needs to be" is repeated across adjacent sentences. Consider rephrasing for variety.
Consider this revision:
-> **Note:** The app cannot run in the tvOS simulator. To test the app, a physical device running tvOS 17.0 or later must be [paired with Xcode](https://support.apple.com/en-us/101262).NetbirdNetworkExtension/NetBirdAdapter.swift (2)
60-63: Consider making the default management URL configurable.The hardcoded public NetBird server URL may limit users who want to connect to self-hosted instances. While
loginAsync()does attempt to extract a stored URL from config, a more explicit configuration mechanism would improve flexibility.
484-496: Regex-based JSON parsing is fragile.Using regex to extract
ManagementURLfrom JSON (line 488) can fail or produce incorrect results with:
- Malformed JSON
- Escaped quotes within the value
- The field appearing in nested structures or JSON embedded in strings
While this works for simple, well-formed config JSON, proper JSON deserialization would be more robust and maintainable.
Consider using
JSONDecoderorJSONSerializationto parse the config structure instead:static func extractManagementURL(from configJSON: String) -> String? { guard let data = configJSON.data(using: .utf8), let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], let url = json["ManagementURL"] as? String, !url.isEmpty else { return nil } return url }NetbirdKit/NetworkExtensionAdapter.swift (1)
338-338: Optional: Remove redundant void return.SwiftLint flags the explicit
-> Voidreturn type as redundant. Functions without a return type implicitly returnVoid, so this can be simplified tofunc stop().
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
NetBird.xcodeproj/project.pbxproj(25 hunks)NetBird/Source/App/Platform/Platform.swift(1 hunks)NetBird/Source/App/ViewModels/MainViewModel.swift(9 hunks)NetBird/Source/App/Views/ServerView.swift(2 hunks)NetBird/Source/App/Views/TV/TVServerView.swift(1 hunks)NetBirdTVNetworkExtension/PacketTunnelProvider.swift(1 hunks)NetbirdKit/NetworkExtensionAdapter.swift(6 hunks)NetbirdKit/Preferences.swift(1 hunks)NetbirdNetworkExtension/NetBirdAdapter.swift(4 hunks)NetbirdNetworkExtension/PacketTunnelProvider.swift(10 hunks)README.md(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- NetBird/Source/App/Views/ServerView.swift
- NetBird/Source/App/Platform/Platform.swift
🧰 Additional context used
🧬 Code graph analysis (3)
NetbirdNetworkExtension/PacketTunnelProvider.swift (2)
NetbirdNetworkExtension/NetBirdAdapter.swift (3)
stop(478-480)start(296-323)login(331-334)NetBird/Source/App/ViewModels/RoutesViewModel.swift (1)
deselectRoute(76-82)
NetBird/Source/App/Views/TV/TVServerView.swift (2)
NetbirdKit/Device.swift (1)
getName(11-17)NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
clearErrorsFor(298-310)loginWithSetupKey(229-296)changeManagementServerAddress(144-206)
NetBird/Source/App/ViewModels/MainViewModel.swift (4)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
onError(24-26)onSuccess(20-22)NetbirdNetworkExtension/NetBirdAdapter.swift (5)
onError(36-38)onError(53-55)onSuccess(40-42)onSuccess(49-51)start(296-323)NetbirdKit/Preferences.swift (2)
newPreferences(49-63)removeConfigFromUserDefaults(120-126)NetBird/Source/App/ViewModels/RoutesViewModel.swift (1)
getRoutes(55-60)
🪛 LanguageTool
README.md
[style] ~96-~96: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...sical device running tvOS 17.0 or later needs to be [paired with Xcode](https://support....
(REP_NEED_TO_VB)
🪛 markdownlint-cli2 (0.18.1)
README.md
63-63: Bare URL used
(MD034, no-bare-urls)
105-105: Bare URL used
(MD034, no-bare-urls)
🪛 SwiftLint (0.57.0)
NetbirdNetworkExtension/PacketTunnelProvider.swift
[Warning] 109-109: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 113-113: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
NetbirdKit/NetworkExtensionAdapter.swift
[Warning] 338-338: Returning Void in a function declaration is redundant
(redundant_void_return)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift
[Warning] 149-149: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 153-153: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 257-257: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 276-276: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 285-285: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 294-294: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 314-314: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 324-324: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 331-331: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 331-331: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 335-335: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 388-388: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
🔇 Additional comments (51)
README.md (1)
57-57: Verify tvOS version requirement consistency.Line 57 specifies tvOS 17.0+, but the PR objectives reference tvOS 16.0+ as the target. Please clarify which version is the actual requirement and ensure consistency across documentation.
NetBird.xcodeproj/project.pbxproj (1)
973-973: Bundle identifier consistency verified.The bundle identifier mismatch flagged in previous reviews has been resolved. Both Debug and Release configurations now use consistent identifiers:
- NetBird TV:
io.netbird.app.tv(lines 973, 1012)- NetBirdTVNetworkExtension:
io.netbird.app.tv.extension(lines 1049, 1086)This ensures proper code signing and provisioning profile compatibility across build configurations.
Also applies to: 1012-1012, 1049-1049, 1086-1086
NetbirdNetworkExtension/NetBirdAdapter.swift (4)
16-56: LGTM! Clean delegation wrapper classes.The login flow helper classes provide a clean bridge between the SDK's Objective-C protocols and Swift closures. The implementation is straightforward and safe.
87-131: LGTM! Platform-specific tunnel FD discovery is well-structured.The conditional compilation cleanly separates iOS and tvOS implementations, with appropriate logging for debugging. The iOS path uses standard SDK APIs while tvOS delegates to a custom implementation.
219-268: LGTM! Failable initializer properly handles platform differences.The initializer now uses
guard letinstead of force unwrapping (addressing the previous review comment), and gracefully handles initialization failures by returningnil. The platform-specific logic correctly handles tvOS's filesystem restrictions by loading config from UserDefaults instead of files.
296-323: LGTM! Proper validation of tunnel file descriptor.The
guardstatement now properly validates the file descriptor, checking for bothniland invalid values (fd <= 0), and returns a descriptive error instead of silently passing invalid data to the SDK. This addresses the previous review concern.NetbirdNetworkExtension/PacketTunnelProvider.swift (7)
23-25: LGTM: Defensive optional adapter initialization.Making the adapter optional allows graceful handling of initialization failures throughout the lifecycle methods. This pairs well with the new guard statements added across the file.
52-60: LGTM: Proper initialization check.The guard ensures the adapter is available before proceeding with the tunnel startup. The error code 1003 and message are clear and actionable.
162-169: LGTM: Safe network change handling.The optional chaining ensures the restart logic handles a nil adapter gracefully without crashing.
171-179: LGTM: Guarded login flow.The guard ensures the adapter is available before attempting login, preventing nil dereference.
181-248: LGTM: Status retrieval with local state caching.The guard protects against nil adapter, and extracting
clientStateto a local variable is a clean refactor that avoids repeated property access.
250-293: LGTM: Guarded route selection.The guard ensures adapter availability before retrieving routes.
295-311: LGTM: Route selection operations guarded.Both functions guard against nil adapter with early returns, maintaining consistency with the rest of the file.
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (11)
15-27: LGTM: Clean SSO listener implementation.The ConfigInitSSOListener follows a standard callback pattern to bridge SDK results to async Swift code.
43-105: LGTM: Comprehensive tvOS startup flow with extensive logging.The startup sequence correctly handles tvOS sandbox restrictions by skipping file-based logging and checking UserDefaults for config. The extensive logging with
privacy: .publicis appropriate for debugging tvOS issues where Console.app is the primary diagnostic tool.Minor question: Why the 2-second delay before returning login error?
Line 86 introduces a 2-second delay before calling the completion handler with a "Login required" error. This seems arbitrary. If there's a specific reason (e.g., waiting for UI to be ready), consider documenting it. Otherwise, the delay could be removed or reduced.
233-259: LGTM: tvOS config transfer via IPC.This function implements the necessary workaround for tvOS where shared UserDefaults between app and extension don't work. Saving to extension-local
UserDefaults.standardand loading directly into the SDK client is the correct approach. The device name update ensures tvOS devices are properly identified.
261-278: LGTM: Simple config storage helpers for tvOS.These functions provide a clean interface for extension-local config storage using
UserDefaults.standard, which is the only reliable storage mechanism available in the tvOS extension sandbox.
280-342: LGTM: Config initialization with SDK file write.This function properly initializes the config file by delegating file writes to the SDK (which has the necessary permissions). Checking UserDefaults for an existing management URL before defaulting is a nice touch that preserves user preferences across initialization.
344-381: LGTM: Platform-specific config initialization.The tvOS path correctly delegates config loading to the adapter, while the iOS path uses a semaphore-based synchronous wait during startup. The 10-second timeout prevents hanging indefinitely.
383-426: LGTM: Detailed login status diagnostics.The diagnostic response format provides comprehensive state information for debugging the tvOS device auth flow. The comment on line 418 correctly notes that
client.isLoginComplete()doesn't work withAuth.Login(), so checkinglastLoginResultis the right approach.
428-512: LGTM: Well-designed tvOS device authentication flow.The loginTV implementation is well thought out:
- Forces device auth (appropriate for tvOS)
- Returns both URL and user code in a pipe-separated format
- Uses a lock to prevent double-completion
- Distinguishes between errors before and after the URL is sent (important for UX)
- Extensive logging aids debugging in the sandbox environment
514-581: LGTM: Status retrieval with proper error handling.The getStatus implementation follows the same pattern as the iOS version, with appropriate guards and a fallback to default status if encoding fails.
583-646: LGTM: Route selection with consistent error handling.The route selection methods follow the established pattern with proper guards and logging. The encoding fallback in getSelectRoutes ensures a response is always returned.
666-710: LGTM: Logging initialization for tvOS extension.The initializeLogging function sets up file-based logging in the App Group container, with appropriate error handling for file operations.
NetBird/Source/App/Views/TV/TVServerView.swift (5)
21-48: LGTM: Well-organized tvOS view structure.The view setup is clean with appropriate use of
@EnvironmentObject,@StateObject, and@FocusStatefor tvOS focus management. The initialization ofServerViewModelwith device name and config path is correct.
49-183: LGTM: Comprehensive form UI with proper focus management.The two-column layout with form fields on the left is well-suited for tvOS. Error messages are displayed per-field, and the optional setup key section is properly gated with clear UI feedback.
184-301: LGTM: Action buttons with proper state handling.The button layout provides clear actions (Cancel, Use NetBird, Change) with disabled states and loading indicators. The lifecycle hooks properly react to SSO errors and successful operations. The two-parameter
onChangesyntax on lines 289 and 294 is correct for the tvOS 17+ target.
305-353: LGTM: Clean async action methods.The action methods properly trim inputs, default to the NetBird URL when needed, clear errors, and call the appropriate async methods on
ServerViewModel. The use ofTaskwithawait Task.yield()is correct for SwiftUI async operations.
355-380: LGTM: Clean helper view and preview.The
InfoRowhelper provides consistent formatting for the info panel, and the preview provider aids development.NetbirdKit/Preferences.swift (6)
11-41: Excellent: Comprehensive architecture documentation.The extensive inline documentation explaining the tvOS config storage architecture is outstanding. It clearly documents the sandbox restrictions, the IPC-based workaround, and contrasts it with iOS behavior. This will be invaluable for future maintainers.
43-63: LGTM: Safe preferences creation with proper nil handling.The optional return type and guard statements address the previous review concern about force unwraps. The function now safely handles cases where the app group container is unavailable, which is important for tvOS.
65-81: LGTM: Safe file path retrieval.The guard statements and optional returns properly handle cases where the app group container is unavailable, addressing the force unwrap concerns from the previous review. The error messages provide clear diagnostic information.
83-101: LGTM: UserDefaults config storage for tvOS.The function provides a clean API for saving config to UserDefaults, which is the tvOS workaround for file write restrictions. The boolean return value allows callers to handle failures appropriately.
103-126: LGTM: Consistent UserDefaults helpers.The load, check, and remove methods follow a consistent pattern with proper nil handling for unavailable UserDefaults.
128-145: LGTM: Config restoration from UserDefaults to file.This function provides the bridge from UserDefaults storage to the file system when needed. The error handling with boolean return is appropriate. Note that this will only work in contexts where file writes are permitted (e.g., the extension).
NetbirdKit/NetworkExtensionAdapter.swift (8)
15-27: LGTM: SSO listener for config initialization.The ConfigSSOListener follows the standard callback pattern for bridging SDK results to Swift code.
29-66: LGTM: Platform-specific extension configuration.The conditional compilation properly sets different extension IDs for tvOS vs iOS. The addition of the
userCodeproperty supports the tvOS device authentication flow.
72-142: LGTM: VPN start sequence with tvOS config initialization.The start sequence properly logs each step for debugging. The tvOS-specific config initialization on lines 120-126 ensures config is available before checking if login is required.
144-219: LGTM: tvOS config initialization with IPC fallback.The
initializeConfigFromAppfunction implements the necessary workaround for tvOS by checking UserDefaults and sending config via IPC. TheinitializeConfigViaExtensionhelper allows requesting default config creation from the extension when it has write permissions the app lacks.
221-299: LGTM: Platform-specific login requirement checks.The tvOS implementation properly checks UserDefaults and then verifies with the management server, which is important for detecting expired tokens. The iOS implementation uses file-based config with SDK verification. The extensive logging aids debugging.
342-385: LGTM: Login flow with tvOS device authentication.The login function properly handles the tvOS device auth flow by sending "LoginTV" and parsing the "url|userCode" response format. Storing the userCode on the main thread is correct for UI binding.
387-481: LGTM: Robust login status checking with user-friendly errors.The
checkLoginCompletefunction robustly parses diagnostic responses in multiple formats for backward compatibility. ThecheckLoginErrorfunction transforms technical error messages into user-friendly guidance, which significantly improves the UX.
601-670: LGTM: IPC-based config transfer for tvOS.The
sendConfigToExtensionandclearExtensionConfigfunctions provide the necessary IPC mechanism for config management on tvOS. The error handling and completion callbacks are appropriate.NetBird/Source/App/ViewModels/MainViewModel.swift (8)
17-42: LGTM: SDK callback listeners.The
SSOCheckListenerandSetupKeyErrListenerclasses provide clean bridges from NetBirdSDK callbacks to Swift closures.
103-114: Excellent: Lazy preferences loading improves startup performance.The lazy loading of preferences avoids expensive SDK initialization (WireGuard/SSH key generation) at app startup. This is particularly important on tvOS where the SDK initialization is costly. The pattern is clean and well-commented.
123-137: LGTM: Deferred Rosenpass settings loading.Deferring the Rosenpass settings load is consistent with the lazy preferences strategy. Since these settings trigger expensive SDK initialization, loading them only when the settings view is accessed is the right trade-off.
148-165: LGTM: Enhanced connect with logging and button lock.The connect method now includes logging for debugging and a 3-second button lock to prevent rapid repeated taps. The async Task block properly awaits the network extension start.
230-261: LGTM: tvOS-specific state text mapping.The tvOS branch properly maps NEVPNStatus to human-readable text since tvOS doesn't use the iOS CustomLottieView. Fetching routes when connected (line 245) ensures the Networks counter on the home screen is accurate.
263-276: LGTM: Comprehensive logout cleanup for tvOS.The clearDetails method properly cleans up UserDefaults config and extension-local config on tvOS, preventing stale credentials from persisting after logout.
278-374: Excellent: Defensive preference methods.All preference-related methods now guard against nil preferences and return safe defaults where appropriate. This prevents crashes when the SDK fails to initialize (which can happen on tvOS) and aligns with the lazy loading strategy.
376-393: LGTM: Platform-appropriate relay connection defaults.The forced relay connection defaults to true on iOS (for battery optimization) and false on tvOS (where battery optimization is not needed). The comment clearly explains the rationale.
README.md
Outdated
|
|
||
| ## gomobile-fork | ||
|
|
||
| Since gomobile doesn't natively support tvOS targets, NetBird has created a fork that does. Please see the repo for more information: https://github.com/netbirdio/gomobile-tvos-fork |
There was a problem hiding this comment.
Wrap bare URLs in markdown link format.
Per markdownlint (MD034), bare URLs should be wrapped in markdown link syntax. Both lines contain unformatted URLs that should be formatted as [URL text](URL).
Apply this diff to fix the bare URL formatting:
-Since gomobile doesn't natively support tvOS targets, NetBird has created a fork that does. Please see the repo for more information: https://github.com/netbirdio/gomobile-tvos-fork
+Since gomobile doesn't natively support tvOS targets, NetBird has created a fork that does. Please see the repo for more information: [https://github.com/netbirdio/gomobile-tvos-fork](https://github.com/netbirdio/gomobile-tvos-fork)Also fix line 105:
-iOS/tvOS Client: https://github.com/netbirdio/ios-client (this repository)
+iOS/tvOS Client: [https://github.com/netbirdio/ios-client](https://github.com/netbirdio/ios-client) (this repository)Also applies to: 105-105
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
63-63: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In README.md around lines 63 and 105, both lines contain bare URLs and violate
markdownlint MD034; replace the raw URLs with markdown link syntax by wrapping
each URL in [text](URL) form — for example use descriptive link text like
"gomobile tvOS fork" or the URL itself as text in square brackets — and update
both lines so they no longer contain bare URLs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Around line 57-96: The workflow step "Create dummy GoogleService-Info.plist"
currently uses working-directory: ios-client which doesn't exist; update that
step (the job named "Create dummy GoogleService-Info.plist" or the block
containing working-directory: ios-client) to remove the working-directory
directive or change it to working-directory: . so the cat heredoc writes
GoogleService-Info.plist into the repository root where NetBird.xcodeproj and
the iOS sources live.
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
19-21: Xcode version 26.1.1 does not exist.This was flagged in a previous review but appears unresolved. Xcode versions are currently in the 15.x-16.x range. This will cause the workflow to fail.
🐛 Proposed fix
- - name: Select Xcode 26.1.1 - run: sudo xcode-select -s /Applications/Xcode_26.1.1.app/Contents/Developer + - name: Select Xcode 16.2 + run: sudo xcode-select -s /Applications/Xcode_16.2.app/Contents/Developer#!/bin/bash # Verify available Xcode versions on macos-15 runners curl -s https://raw.githubusercontent.com/actions/runner-images/main/images/macos/macos-15-Readme.md | grep -A 20 "Xcode"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:54.967Z
Learning: On iOS, the pre-shared key (PSK) is stored in the SDK's config file, not in the Keychain. On tvOS, the config (including PSK) is stored in UserDefaults and transferred to the network extension via IPC as a single JSON blob, making Keychain integration architecturally more complex.
🔇 Additional comments (3)
.github/workflows/test.yml (3)
16-16: LGTM!The runner update to
macos-15is appropriate for the tvOS support additions in this PR.
39-45: LGTM!The gomobile-netbird installation and initialization steps are correctly structured. The PATH export ensures the
gomobile-netbirdbinary is discoverable.
47-51: LGTM!The PATH export before running
build-go-lib.shensures the Go binaries installed in the previous step are available.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@build-go-lib.sh`:
- Around line 19-20: The script uses the wrong gomobile binary and omits tvOS
targets: replace the calls to gomobile (the lines invoking "gomobile init" and
"gomobile bind" in build-go-lib.sh) with the forked tool "gomobile-netbird" and
update the bind target flag on the NetBirdSDK bind command from
"-target=ios,iossimulator" to "-target=ios,iossimulator,tvos,tvossimulator".
Also update CI install steps in .github/workflows/build.yml and
.github/workflows/test.yml by replacing the current "go install
golang.org/x/mobile/cmd/gomobile@latest" with "go install
github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest" so the
workflows install the correct forked binary.
♻️ Duplicate comments (2)
.github/workflows/build.yml (2)
119-145: Add build steps for tvOS targets.This PR adds tvOS support with
NetBird TVandNetBirdTVNetworkExtensiontargets, but the CI workflow only builds the iOS app and extension. Add build steps for the tvOS targets to ensure they compile successfully.Proposed additions after the iOS build steps
- name: Build tvOS App working-directory: ios-client run: | set -o pipefail xcodebuild build \ -project NetBird.xcodeproj \ -scheme "NetBird TV" \ -destination 'generic/platform=tvOS' \ -configuration Debug \ CODE_SIGNING_ALLOWED=NO \ CODE_SIGNING_REQUIRED=NO \ CODE_SIGN_IDENTITY="" \ | xcpretty --color - name: Build tvOS Network Extension working-directory: ios-client run: | set -o pipefail xcodebuild build \ -project NetBird.xcodeproj \ -scheme NetBirdTVNetworkExtension \ -destination 'generic/platform=tvOS' \ -configuration Debug \ CODE_SIGNING_ALLOWED=NO \ CODE_SIGNING_REQUIRED=NO \ CODE_SIGN_IDENTITY="" \ | xcpretty --colorRun the following to verify the tvOS scheme names:
#!/bin/bash # Find available schemes in the Xcode project xcodebuild -project ios-client/NetBird.xcodeproj -list 2>/dev/null | grep -A 50 "Schemes:" || \ rg -i "NetBird.*TV|tvos" ios-client/NetBird.xcodeproj/project.pbxproj | head -20
19-20: Xcode 26.1.1 is not available on the macos-15 runner.The macos-15 GitHub Actions runner (as of January 2026) includes Xcode 26.2, not 26.1.1. This step will fail because
/Applications/Xcode_26.1.1.appdoes not exist. Update to use the installed version.Proposed fix
- - name: Select Xcode 26.1.1 - run: sudo xcode-select -s /Applications/Xcode_26.1.1.app/Contents/Developer + - name: Select Xcode 26.2 + run: sudo xcode-select -s /Applications/Xcode_26.2.app/Contents/Developer
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
40-40: Consider pinning gomobile to a specific version for reproducible builds.Using
@latestcan lead to non-reproducible builds if gomobile introduces breaking changes. Pin to a specific version or commit hash to ensure consistent CI behavior.Proposed fix
- run: go install golang.org/x/mobile/cmd/gomobile@latest + run: go install golang.org/x/mobile/cmd/[email protected]Replace the commit hash with the latest known-working version.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build.yml.github/workflows/test.ymlbuild-go-lib.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test
- GitHub Check: Build iOS App
🔇 Additional comments (1)
.github/workflows/build.yml (1)
72-110: LGTM!Creating a dummy
GoogleService-Info.plistfor CI builds is a sensible approach to allow compilation without real Firebase credentials. The plist structure is correct and all Firebase features are appropriately disabled.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
build-go-lib.sh
Outdated
| gomobile init | ||
| CGO_ENABLED=0 gomobile bind -target=ios -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBird/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK | ||
| CGO_ENABLED=0 gomobile bind -target=ios,iossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking build-go-lib.sh ==="
cat build-go-lib.sh 2>/dev/null || echo "File not found at root"
# Search for build-go-lib.sh
echo -e "\n=== Finding build-go-lib.sh ==="
find . -name "build-go-lib.sh" -type f 2>/dev/null | head -5Repository: netbirdio/ios-client
Length of output: 614
🏁 Script executed:
#!/bin/bash
echo "=== Checking CI workflows for gomobile references ==="
echo "--- .github/workflows/test.yml ---"
grep -n -i "gomobile" .github/workflows/test.yml 2>/dev/null || echo "No gomobile reference found"
echo -e "\n--- .github/workflows/build.yml ---"
grep -n -i "gomobile" .github/workflows/build.yml 2>/dev/null || echo "No gomobile reference found"
echo -e "\n--- All workflow files ---"
find .github/workflows -name "*.yml" -o -name "*.yaml" 2>/dev/null | xargs grep -l "gomobile" 2>/dev/null || echo "No gomobile found in any workflows"Repository: netbirdio/ios-client
Length of output: 474
🏁 Script executed:
#!/bin/bash
echo "=== Searching for gomobile fork references ==="
grep -r "gomobile-netbird" . --include="*.md" --include="*.sh" --include="*.yml" --include="*.yaml" 2>/dev/null | head -20
echo -e "\n=== Searching for tvOS references ==="
grep -r -i "tvos" . --include="*.md" --include="*.sh" --include="*.go" --include="*.yml" 2>/dev/null | head -20
echo -e "\n=== Searching for gomobile fork setup ==="
grep -r "gomobile" . --include="*.sh" --include="*.md" --include="*.yml" 2>/dev/null | grep -v "Binary file" | head -30Repository: netbirdio/ios-client
Length of output: 3124
🏁 Script executed:
#!/bin/bash
echo "=== Checking README for gomobile documentation ==="
grep -A 5 -B 5 -i "gomobile" README.md 2>/dev/null | head -50
echo -e "\n=== Checking if there's a setup or installation doc ==="
find . -name "README*" -o -name "SETUP*" -o -name "INSTALL*" | head -10Repository: netbirdio/ios-client
Length of output: 1187
🏁 Script executed:
#!/bin/bash
echo "=== Listing .github/workflows directory ==="
ls -la .github/workflows/ 2>/dev/null || echo "Workflows directory not found"
echo -e "\n=== Checking top-level files ==="
ls -la | grep -E "\.(md|sh|yml|yaml)$"Repository: netbirdio/ios-client
Length of output: 568
Fix script to use gomobile-netbird and add tvOS targets; fix CI workflows.
The script uses standard gomobile instead of gomobile-netbird, which doesn't support tvOS. Additionally, the targets are missing tvOS and tvossimulator, and the CI workflows are installing the wrong gomobile version.
Required fixes:
- Replace
gomobilewithgomobile-netbirdon lines 19-20 - Change
-target=ios,iossimulatorto-target=ios,iossimulator,tvos,tvossimulator - Update
.github/workflows/build.ymland.github/workflows/test.ymlto install the correct fork:- Replace:
go install golang.org/x/mobile/cmd/gomobile@latest - With:
go install github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest
- Replace:
The README correctly documents these requirements but the implementation does not follow them.
Proposed fix for build-go-lib.sh
cd $netbirdPath
-gomobile init
-CGO_ENABLED=0 gomobile bind -target=ios,iossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK
+gomobile-netbird init
+CGO_ENABLED=0 gomobile-netbird bind -target=ios,iossimulator,tvos,tvossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK🤖 Prompt for AI Agents
In `@build-go-lib.sh` around lines 19 - 20, The script uses the wrong gomobile
binary and omits tvOS targets: replace the calls to gomobile (the lines invoking
"gomobile init" and "gomobile bind" in build-go-lib.sh) with the forked tool
"gomobile-netbird" and update the bind target flag on the NetBirdSDK bind
command from "-target=ios,iossimulator" to
"-target=ios,iossimulator,tvos,tvossimulator". Also update CI install steps in
.github/workflows/build.yml and .github/workflows/test.yml by replacing the
current "go install golang.org/x/mobile/cmd/gomobile@latest" with "go install
github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest" so the
workflows install the correct forked binary.
Capture full xcodebuild output and system crash logs when tests fail. This will help debug the test crash that occurs during app bootstrapping in CI but not locally. - Tee test output to file before xcpretty filtering - Search for and print crash reports from DiagnosticReports - Upload test logs and crash reports as artifacts on failure
build-go-lib.sh
Outdated
|
|
||
| gomobile init | ||
| CGO_ENABLED=0 gomobile bind -target=ios -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBird/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK | ||
| CGO_ENABLED=0 gomobile bind -target=ios,iossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK |
There was a problem hiding this comment.
Shouldn't this be changed to gomobile-netbird?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Line 16: The workflow is selecting a non-existent Xcode version (26.1.1) for
the macos-15 runner; update the Xcode selection in .github/workflows/test.yml
where you set the Xcode version (look for the XCODE_VERSION/matrix entry or the
step input that uses an action to setup Xcode and the literal "26.1.1") to a
valid 16.x value (e.g., "16.0" or "16.1"), and ensure any matrix or env variable
referencing "26.1.1" is changed accordingly so the macos-15 runner can
successfully find the Xcode toolchain.
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
19-20: Xcode 26.1.1 path looks invalid on GitHub runners.
This will fail if that bundle doesn’t exist; please switch to a version that is actually installed on the macos-15 image.GitHub Actions macos-15 runner image preinstalled Xcode versions list
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml.github/workflows/test.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:54.967Z
Learning: On iOS, the pre-shared key (PSK) is stored in the SDK's config file, not in the Keychain. On tvOS, the config (including PSK) is stored in UserDefaults and transferred to the network extension via IPC as a single JSON blob, making Keychain integration architecturally more complex.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test
- GitHub Check: Build iOS App
🔇 Additional comments (3)
.github/workflows/test.yml (3)
53-92: LGTM: dummy GoogleService-Info.plist generation.
This keeps CI unblocked without embedding real credentials.
109-150: Nice: richer test artifacts on failure.
Capturing xcresult, logs, and crash reports should significantly improve triage.
39-43: Thegobind-netbirdinstallation can be safely commented out. Search results show no active invocations of thegobind-netbirdtool in any build scripts—only references appear in pre-generated header file comments that are already committed to the repository.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…y nicely with github actions
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Line 136: The workflow currently uses an invalid simulator destination string
'-destination 'platform=iOS Simulator,name=iPhone 16,OS=18.5'' which won't exist
for the chosen Xcode; update the workflow to select a valid Xcode (e.g., Xcode
16.x) and change the destination to a simulator that the selected Xcode provides
(use the existing "List available simulators" step output to pick a matching
'name' and 'OS' pair), or implement a small fallback that picks the first
matching iOS simulator from `xcrun simctl list` if the explicit destination is
not found.
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
19-20: Xcode 26.1.1 does not exist — workflow will fail.This issue was previously flagged but remains unaddressed. The macos-15 runner only includes Xcode 16.x versions. Update to a valid version such as
Xcode_16.2.apporXcode_16.1.app.- - name: Select Xcode 26.1.1 - run: sudo xcode-select -s /Applications/Xcode_26.1.1.app/Contents/Developer + - name: Select Xcode 16.2 + run: sudo xcode-select -s /Applications/Xcode_16.2.app/Contents/Developer
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build iOS App
- GitHub Check: Build and Test
🔇 Additional comments (6)
.github/workflows/test.yml (6)
39-47: LGTM!The
go installcommands are now correctly specified, and the GOPROXY=direct approach ensures direct fetching from the fork repository.
57-95: LGTM!The
ios-clientdirectory now exists via the checkout step (line 25), resolving the previously flagged working directory issue. The dummy plist provides all required Firebase configuration keys for the build to succeed.
104-127: LGTM!The environment and simulator introspection steps provide valuable diagnostic information for CI debugging. The App Group container check helps verify expected sandbox limitations.
142-163: LGTM!The failure handling steps provide comprehensive test result extraction using
xcresulttooland proper artifact uploads. This will significantly aid debugging when tests fail.
165-192: LGTM!The crash log collection is comprehensive, searching multiple locations (DiagnosticReports, CoreSimulator, system.log) with appropriate time filters. The
if-no-files-found: ignoreprevents workflow failures when no crashes occur.
36-36: Go 1.24 is available and released.Go 1.24 was released on February 11, 2025, and is available via
actions/setup-go@v5. No changes needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
.github/workflows/test.yml
Outdated
| -project NetBird.xcodeproj \ | ||
| -scheme NetBird \ | ||
| -destination 'platform=iOS Simulator,name=iPhone 16' \ | ||
| -destination 'platform=iOS Simulator,name=iPhone 16,OS=18.5' \ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "test.yml" -path "*/workflows/*" -type fRepository: netbirdio/ios-client
Length of output: 92
🏁 Script executed:
cat -n .github/workflows/test.yml | sed -n '1,150p'Repository: netbirdio/ios-client
Length of output: 6022
🏁 Script executed:
# Check lines around the test step to see Xcode version selection
cat -n .github/workflows/test.yml | sed -n '120,145p'Repository: netbirdio/ios-client
Length of output: 1134
Verify the simulator destination works with the selected Xcode version.
The destination iPhone 16,OS=18.5 depends on the Xcode version's available simulators. Since Xcode 26.1.1 doesn't exist, this test will fail at the Xcode selection step. Once corrected to a valid Xcode version (16.x or similar), verify that this iOS version and device combination are available. The workflow already includes a "List available simulators" step that will show what's actually available on the runner—if the specified simulator doesn't exist, the test will fail without a fallback.
🤖 Prompt for AI Agents
In @.github/workflows/test.yml at line 136, The workflow currently uses an
invalid simulator destination string '-destination 'platform=iOS
Simulator,name=iPhone 16,OS=18.5'' which won't exist for the chosen Xcode;
update the workflow to select a valid Xcode (e.g., Xcode 16.x) and change the
destination to a simulator that the selected Xcode provides (use the existing
"List available simulators" step output to pick a matching 'name' and 'OS'
pair), or implement a small fallback that picks the first matching iOS simulator
from `xcrun simctl list` if the explicit destination is not found.
shuuri-labs
force-pushed
the
feat/add-tvos-support-beta
branch
from
6fc6ea6 to
42e844d
Compare
…d config so always appears as disabled
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Around line 148-154: The "Upload crash reports" workflow step uses a tilde in
the path which may not expand; update the step to use an explicit $HOME
expansion or a known artifacts directory: change the path value from
"~/Library/Logs/DiagnosticReports/*.crash" to
"$HOME/Library/Logs/DiagnosticReports/*.crash" or add a prior "Collect crash
reports" step that copies files to a local directory (e.g., crash-reports/) and
then set the "Upload crash reports" step's path to that directory; ensure the
step name "Upload crash reports" and its path field are the ones updated.
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
21-22: Xcode version 26.1.1 does not exist.This issue was previously flagged. The macos-15 runner only includes Xcode 16.x versions. Update to a valid version like
Xcode_16.2.appor similar.
| - name: Upload crash reports | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: crash-reports | ||
| path: ~/Library/Logs/DiagnosticReports/*.crash | ||
| if-no-files-found: ignore |
There was a problem hiding this comment.
Tilde (~) may not expand correctly in upload-artifact path.
The path: ~/Library/Logs/DiagnosticReports/*.crash uses tilde for home directory, but actions/upload-artifact may not expand ~ correctly. Use the $HOME environment variable or an absolute path instead.
🐛 Proposed fix
- name: Upload crash reports
if: failure()
uses: actions/upload-artifact@v4
with:
name: crash-reports
- path: ~/Library/Logs/DiagnosticReports/*.crash
+ path: ${{ runner.temp }}/../../../Library/Logs/DiagnosticReports/*.crash
if-no-files-found: ignoreAlternatively, copy the crash logs to a known location in a prior step:
- name: Collect crash reports
if: failure()
run: |
mkdir -p crash-reports
cp ~/Library/Logs/DiagnosticReports/*.crash crash-reports/ 2>/dev/null || true
- name: Upload crash reports
if: failure()
uses: actions/upload-artifact@v4
with:
name: crash-reports
path: crash-reports/
if-no-files-found: ignore🤖 Prompt for AI Agents
In @.github/workflows/test.yml around lines 148 - 154, The "Upload crash
reports" workflow step uses a tilde in the path which may not expand; update the
step to use an explicit $HOME expansion or a known artifacts directory: change
the path value from "~/Library/Logs/DiagnosticReports/*.crash" to
"$HOME/Library/Logs/DiagnosticReports/*.crash" or add a prior "Collect crash
reports" step that copies files to a local directory (e.g., crash-reports/) and
then set the "Upload crash reports" step's path to that directory; ensure the
step name "Upload crash reports" and its path field are the ones updated.
This commit introduces full tvOS support for the NetBird iOS client, enabling VPN connectivity on Apple TV devices.
Added NetBird TV app target with tvOS 16.0+ deployment
Created tvOS-specific UI using SwiftUI optimized for "10-foot experience"
Tab-based navigation: Connection, Peers, Networks, Settings
Large touch targets and text for Siri Remote navigation
TVMainView: Main tab navigation and connection status
TVConnectionView: Large connect/disconnect button with status display
TVPeersView: List of connected peers with connection details
TVNetworksView: Network routes selection and management
TVSettingsView: App settings and logout functionality
TVAuthView: QR code + device code authentication flow
Implemented OAuth device authorization flow for tvOS
Displays QR code that user scans with phone to authenticate
Shows user code as fallback for manual entry
Polls for authentication completion and auto-dismisses on success
tvOS has stricter sandbox restrictions than iOS:
UserDefaults-based Config Storage
Preloaded Config in Go SDK
Raw Syscall Tunnel FD Discovery
Added NetBirdTVNetworkExtension target
Separate PacketTunnelProvider.swift with tvOS-specific handling
Extensive logging for debugging via Console.app
Handles "LoginTV" message for device auth flow
Loads config from UserDefaults into SDK memory
isLoginRequired() now verifies session with management server
Previously only checked if config existed (caused post-restart failures)
Shows QR code re-auth flow when OAuth session expires
Added Platform.swift for iOS/tvOS conditional compilation
Shared code uses #if os(tvOS) / #if os(iOS) where needed
Common ViewModels work across both platforms
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.